fix(build): support absolute DerivedData paths with xcodemake#485
Merged
Conversation
Update the pinned xcodemake revision so absolute DerivedData paths produce safe capture log filenames. Match the new logs by their argument hash before deciding whether to run make directly. Fixes #466
Always invoke the pinned wrapper so it owns Makefile argument and freshness validation before delegating to make. Refs #466
Execute the checksum-verified pinned wrapper with fake build tools and verify capture, reuse, argument invalidation, and project freshness behavior. Refs #466
commit: |
There was a problem hiding this comment.
Pull request overview
This PR updates XcodeBuildMCP’s incremental-build path to always invoke the pinned xcodemake wrapper (instead of locally deciding when to run make), and refreshes the pinned wrapper commit/checksum to address failures when -derivedDataPath is an absolute external path.
Changes:
- Updated the pinned
xcodemakecommit + SHA-256, and added a checksum-verified fixture plus a wrapper lifecycle regression test. - Simplified incremental build execution to always route through
xcodemake, delegating Makefile reuse / invalidation decisions to the wrapper. - Added unit coverage ensuring
executeXcodeBuildCommanddelegates toxcodemake(via mocking) for external absolute DerivedData paths, and recorded the fix in the changelog.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/xcodemake.ts | Updates the pinned xcodemake commit/checksum and keeps download + integrity verification flow. |
| src/utils/build-utils.ts | Removes local Makefile/log heuristics and always runs xcodemake for incremental builds. |
| src/utils/tests/xcodemake.test.ts | Drops coverage for removed local log-file detection helper. |
| src/utils/tests/xcodemake-wrapper.test.ts | Adds an execution-based regression test for the pinned wrapper behavior using hermetic fake build tools. |
| src/utils/tests/fixtures/xcodemake/xcodemake-7d20… | Adds the exact pinned wrapper script as a fixture for checksum-verified regression execution. |
| src/utils/tests/fixtures/xcodemake/README.md | Documents fixture pinning and checksum requirements. |
| src/utils/tests/build-utils-xcodemake.test.ts | Adds a focused unit test verifying build-utils always delegates incremental builds to xcodemake. |
| CHANGELOG.md | Adds an Unreleased entry for the incremental xcodemake absolute DerivedData fix (Issue #466). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Repin the bundled wrapper to the upstream lifecycle fixes and add regression coverage for long configuration arguments, direct fallback, and Makefile reuse.
Keep the wrapper lifecycle test independent of the process working directory.
Compare the observed Makefile timestamp with the explicit sentinel using a filesystem-resolution tolerance.
cameroncooke
marked this pull request as ready for review
July 22, 2026 08:35
cameroncooke
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
xcodemakeis XcodeBuildMCP's optional incremental-build accelerator. On its first run it captures anxcodebuildinvocation and generates a Makefile; later runs can reuse that Makefile to avoid the startup cost of another full Xcode build.When XcodeBuildMCP passed an absolute DerivedData path such as
/Users/.../DerivedData, the bundled wrapper included the raw build arguments in its capture-log filename. The slashes were treated as directory separators, so the log could not be created and the build failed before a Makefile was generated.XcodeBuildMCP also tried to decide for itself whether an existing Makefile was safe to reuse. That check depended on the wrapper's old log naming scheme and could select the project's single Makefile even when it had been generated for different build arguments.
Fix
This PR updates the checksum-pinned
xcodemakewrapper to a repaired revision that converts the build arguments into a bounded, filesystem-safe log name with an argument hash to prevent collisions.Every incremental build now goes through the wrapper. The wrapper owns the decision to reuse or regenerate the Makefile based on the exact build arguments and whether the project has changed. Calls without an explicit project or workspace path use the current working directory, so xcodemake always receives a valid working directory and absolute arguments keep their leading slash.
Standard builds that do not use
xcodemakeare unchanged. The inherited Git configuration anddoctorcommand concerns also mentioned in the issue are separate problems and remain outside this PR.Regression protection
The regression test executes an exact, checksum-verified copy of the production-pinned Perl wrapper against isolated fake
xcodebuildandmakeexecutables. It verifies that:-configurationis preserved during capture and direct-build fallback;makeinvocation follows the wrapper's directxcodebuildrecovery path; andThe focused coverage and full unit suite pass with 2,896 tests, together with type checking, the production build, formatting, and linting. Snapshot and smoke suites were not run because repository policy requires separate approval.
Fixes #466.